Skip to content
This repository has been archived by the owner on Sep 6, 2021. It is now read-only.

New Resizer APIs: show, hide and toggle #1838

Merged
merged 3 commits into from
Oct 19, 2012
Merged

New Resizer APIs: show, hide and toggle #1838

merged 3 commits into from
Oct 19, 2012

Conversation

jbalsas
Copy link
Contributor

@jbalsas jbalsas commented Oct 14, 2012

Hi,

This is a pull request prior to the sidebar refactorization proposed in #1811

It creates three new APIs show hide and toggle on the utils/Resizer module to change the visibility of resizable elements.

It also scans a new class collapsable on the elements to trigger the toggle function when double clicking on the resizer element.

The show and hide APIs are meant to decouple the visualization panels from the actual working of the Modules. For example, once the statusbar indicator actions are implemented, the jslint results panel could be shown or hidden while keeping the scan running.

@ghost ghost assigned redmunds Oct 14, 2012
@@ -75,15 +112,21 @@ define(function (require, exports, module) {
* - Left ("left") or right ("right") for horizontal resizing
*
* A resizable element triggers the following events while resizing:
* - panelResizeStarts: When the resize starts
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo -- the event is "panelResizeStart"

@redmunds
Copy link
Contributor

Looking at the AppInit.htmlReady() calls in FindInFiles.js and JSLintUtils.js, the code looks almost identical. Is there any way to automate that? Or at least simplify it down to a few parameters that can be passed to an API call?

@jbalsas
Copy link
Contributor Author

jbalsas commented Oct 15, 2012

FindInFiles and JSLintUtils are only using the panelResizeEnd. The sidebar will be the first module to be using all the events dispatched from the Resizer module.

I think we could find some solution, but I'm afraid it won't be as flexible as I'd like... Maybe we could translate this to the discussion in the final pull request (once we can see all we'd need) to see how that API could look like?

@redmunds
Copy link
Contributor

Done with initial code review

@jbalsas
Copy link
Contributor Author

jbalsas commented Oct 16, 2012

@redmunds I was thinking about the API for initializing the panels. I think most things could be automated if we allow the Resizer to keep its own track of the sizes and toggle states preferences. When calling the makeResizable API either automatically or manually from an extension, the panel size will be initialized to the value in the preferences or a default one. Then, the module will save the appropriate values on resizePanelEnd, panelExpanded and panelCollapsed and the panels won't need to know about it.

I think this could make it way cleaner for the regular cases. What do you think? Does it make sense to store the size preferences in utils/Resizer instead of inside each particular module or extension?

By the way, I didn't realize you hadn't finished the review when I submitted the changes. Sorry about that.

Please, tell me if there's anything else you'd want to see in here.

@peterflynn
Copy link
Member

@jbalsas: there are some changes in #1847 that will add a little extra work to the next phase of your refactoring (changing SidebarView to use the generic Resizer code). But I don't think it will be too bad: updating the margin via a panelResizeUpdate listener in EditorManager should work fine...

@redmunds
Copy link
Contributor

@jbalsas I agree that storing the sizes in utils/Resizer is cleaner.

No problem about checking in changes while I'm still reviewing. I got side-tracked and should not have taken so long.


// We wait 100ms to remove the resizer container to capture a mousedown
// on the container that would account for double click
setTimeout(function () {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We've been using window.setTimeout to minimize our use of globals in our jslint config.

@jbalsas
Copy link
Contributor Author

jbalsas commented Oct 16, 2012

@jasonsanjose Thanks for pointing it out. Fixed!

@peterflynn We're getting close to the final refactor step. I'll make sure to carefully go through those changes. As you say, hopefully it won't be too hard...

@redmunds I already moved the size preferences to utils/Resizer and as expected it cleans things up quite nicely. I also added the data-minsize attribute we discussed. I have them in a separate branch pending the solution of this pull request to create a new one. Would you prefer if I added those changes here? Also, as you're getting close to the end of the sprint, maybe you prefer to wait to merge any changes. In that case, I could wait to push anything else until then.

@redmunds
Copy link
Contributor

@jbalsas Yes, I think I'll wait until we close down this sprint to merge any more changes.

@redmunds
Copy link
Contributor

Looks good. Merging.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants